feat(chat): quote transcript selections into the composer - #28
feat(chat): quote transcript selections into the composer#28tulsi-builder wants to merge 19 commits into
Conversation
The Markdown renderer now produces canonical source coordinates for rendered text (rehype segment spans + per-Streamdown-block start offsets), and the quote selection mapper intersects the DOM range with those segments instead of guessing via unique-substring matching. This makes selections across numbered list items, bold/link labels, repeated phrases, and multiple Streamdown blocks map losslessly back to canonical Markdown source. The substring fallback remains only for markdown surfaces that have not enabled sourceSegments.
Decision: Option A (Berd-local durable persistence, no backend change). Research confirmed the pinned Goose backend generates user-message ids server-side during reply() and never echoes them to the client in the live turn, so submitted quote metadata on the locally created user message cannot be joined to replayed turns by id. The dispatched prompt text is what Goose persists and replays as the turn's user-visible text, and replay preserves send order — so the join is ordered text matching. sendCore/steerCore record the dispatched prompt text alongside staged quotes at the dispatch boundary; both replay consumers (session activation and post-compaction reload) re-attach the records to replayed user turns. Records never overwrite live staged items, duplicates are consumed in send order, turns removed by compaction simply find no match, and session deletion clears its records.
A selection may now span multiple text blocks and multiple messages (any roles). The mapper intersects the DOM range with every touched text block in document order, clamps the range to each block's edges, maps each slice through renderer source segments (or the plain-text/unique-substring paths), and assembles one quote with one source range per block. Non-text content between blocks (tool cards, images) is clamped out rather than blocking the quote. Presentation counts distinct source messages, so a multi-block selection within one message still reads as a single-message quote.
Quote serialization moves from the composer to dispatch (sendCore and steerCore), where any compaction for the attempt has already run and the live transcript can decide framing per quote source: - anchor framing when every source turn survives: the passage appears verbatim earlier in the conversation, so long excerpts elide to head…tail anchors instead of re-sending the whole passage - full-excerpt framing when a source was compacted away or rewritten shorter than the quoted range The composer now only snapshots structured staged quotes into the send (userMessageMetadata.stagedItems); no assistantPrompt is precomputed. A quote-only send (no typed text) is accepted since the prompt framing is derived at dispatch. The decision is per quote, not a session-wide hasCompacted boolean, per the handoff review.
Adds quotes to ChatInputControls as the single availability decision for transcript quoting: a composer without quotes neither displays staged quote chips nor includes staged quotes in sends. ChatView derives both the transcript quote affordance and the composer control from one quotesEnabled decision instead of two independently drifting switches. Home and the automations builder opt out explicitly — neither surface has a quotable transcript.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison c518c9d...e46f5ae found four blocking correctness issues in quote provenance, queued-message editing, voice submission, and Markdown selection mapping. The supplied GitHub evidence was inspected: all listed check runs completed successfully, while the combined commit status was pending at capture time; required checks still govern merge readiness.
Deterministic publication result: 4 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
| const quotePrompt = buildStagedQuoteDispatchPrompt(stagedItems, (source) => | ||
| stagedQuoteSourceIsLive(liveMessages, source), | ||
| ); | ||
| recordSubmittedStagedItems(sessionId, acpPrompt, stagedItems); |
There was a problem hiding this comment.
🤖 P1 · Record provenance only after dispatch (blocking)
prepareStagedQuoteDispatch persists a submitted-quote record here before acpSendMessage or acpSteerMessage crosses the transport boundary. Both calls can still reject before dispatch; steering then removes its optimistic user message, while a queued send remains eligible for retry, but neither path removes this record. Replay restoration later consumes records by prompt-text matching, so failed attempts remain durable and can be matched to a different accepted turn with the same text.
User effect: After a failed send or steer, a later unquoted message with the same text can incorrectly gain the old quote card after replay. Retrying a queued quote can also accumulate duplicate or stale provenance and consume the per-session record budget.
Recommended fix: Separate quote-prompt construction from provenance persistence. Commit the submitted record only from the authoritative transport-accepted callback, make that commit idempotent per attempt, and record steering only after delivery is acknowledged or otherwise established.
Test: For send and steer, attempt a quoted prompt, reject it before transport acceptance, then successfully send the same text without a quote and replay it; assert that no quote is restored. Also assert that one rejected queued attempt followed by one accepted retry creates exactly one provenance record.
| submittedText, | ||
| submittedAttachments, | ||
| submittedSkills, | ||
| submittedStagedItems, |
There was a problem hiding this comment.
🤖 P1 · Synchronize quotes during queue edits (blocking)
The normal-submit branch passes submittedStagedItems here, but the queued-edit branch bypasses it and reuses restoredQueuedSendOptions without synchronizing the staged-item state displayed by the composer. Opening an edit preserves the queued quote invisibly inside userMessageMetadata while visible chips still come from stagedItemsProp. Saving can therefore retain an unseen queued quote, ignore a different visible quote, and then remove that visible quote during cleanup even though it was never written to the updated queue payload.
User effect: A queued message can later send quote intent the user could not see while editing, while a quote visibly shown in the editor can be discarded. The editor no longer represents the message intent retained by the queue.
Recommended fix: When queue editing begins, hydrate the editor's staged-item source from the queued payload and use that visible snapshot as the single source of truth. On update, write exactly that snapshot into userMessageMetadata and clear only items incorporated into the accepted update.
Test: Edit a queued message containing quote A while the composer contains quote B. Assert that the editor visibly represents the authoritative quote snapshot, that the saved queued payload exactly matches what was displayed, and that no unrelated visible quote is removed without being saved.
| submittedText, | ||
| submittedAttachments, | ||
| submittedSkills, | ||
| stagedItemsRef.current, |
There was a problem hiding this comment.
🤖 P1 · Clear quotes after voice submission (blocking)
Voice auto-submit includes stagedItemsRef.current in the send at this line, but its accepted path clears only selected skills. useVoiceDictation subsequently clears text and attachments, and this path bypasses submitCurrentMessage, which contains the staged-item cleanup used by manual sends. The accepted quote consequently remains staged.
User effect: The sent quote chip remains in the empty composer and is silently included again in the next typed or dictated message unless the user notices and removes it, causing the agent to answer against stale context.
Recommended fix: Apply the same snapshot-aware staged-item cleanup to voice auto-submit as manual submission: after acceptance, remove only the submitted items if the current staged snapshot still matches, preserving any quote added while the send was pending.
Test: Voice-auto-submit valid text with one staged quote, assert the send receives it, resolve acceptance, and assert the submitted chip is removed. Send a second message and assert the first quote is absent. Add a race case where a different quote is staged before acceptance and must remain.
| range: Range, | ||
| ): { start: number; end: number } | null { | ||
| const segments = Array.from( | ||
| block.querySelectorAll(SOURCE_SEGMENT_SELECTOR), |
There was a problem hiding this comment.
🤖 P2 · Preserve unsegmented selection boundaries (blocking)
mapRangeThroughSourceSegments considers only wrapped source segments intersecting the range. Streamdown content such as inline code is intentionally not wrapped by markdown-source-segments. When a selection starts inside inline code and ends in later wrapped prose, the first intersected segment is the later prose; mapping clamps to that segment and returns success instead of accounting for or rejecting the selected unsegmented prefix. The newer head changes only affordance positioning, so this truncation remains present.
User effect: A user can select a code expression plus surrounding prose, but the staged chip and assistant-facing quote silently omit the code at the start. The agent receives different context from what the user selected.
Recommended fix: Treat unsegmented content at either selected boundary as an unresolved gap rather than silently clamping past it. Recover a canonical range only when it can be proven lossless, or decline the quote affordance for that selection; never return a partial quote that omits selected text.
Test: Render Markdown containing inline code through the real renderer, start the DOM selection inside the code expression, end inside following prose, and assert that the staged excerpt includes the code and begins at or before its canonical occurrence. Add the mirrored case where selection ends inside unsegmented content.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
Fresh review of head 36a1c549 found two additional blocking user-facing defects beyond the four existing unresolved review threads. The quote transport itself is now excerpt-first and message-ID-based; the comments in submitComposerMessage.ts, sendCore.ts, and steerCore.ts still describe an anchor-vs-full-excerpt decision that no longer exists and should be corrected, but I am not blocking separately on stale comments. I could not run the suite locally because this review checkout has no node_modules; GitHub CI is still running at publication time.
|
|
||
| const stagePendingQuote = useCallback(() => { | ||
| if (!sessionId || !pendingQuote) return; | ||
| useChatStore.getState().setStagedItems(sessionId, [pendingQuote.item]); |
There was a problem hiding this comment.
P1 · Preserve quotes already staged in the composer
This replaces the session's entire staged-item array with the new quote. That conflicts with the array-shaped composer/dispatch model and even bypasses the store's existing addStagedItem: selecting passage B after passage A silently removes A, so the eventual message carries only the last selection.
Use addStagedItem(sessionId, pendingQuote.item) (or explicitly define and communicate a one-quote limit rather than silently overwriting). Add a user-path test that stages A and then B through this affordance and verifies both chips and both dispatch excerpts remain.
| <p className="shrink-0 border-b border-popover-inverse-foreground/15 px-3 py-2 text-xs text-popover-inverse-foreground/70"> | ||
| {source} · {extent} | ||
| </p> | ||
| <div className="min-h-0 overflow-y-auto px-3 py-2.5"> |
There was a problem hiding this comment.
P2 · Make the complete quote preview keyboard-scrollable
For a long/compacted quote this scroll container is the only path to the full excerpt, but it is not focusable. ComposerChip makes the HoverCard trigger focusable, so a keyboard user can reveal the panel, but focus remains on the trigger and Arrow/Page keys cannot scroll this overflow-y-auto region. The quoted context can therefore be inspected fully with a mouse wheel but not from the keyboard.
Use a keyboard-operable disclosure/popover/dialog, or give the scroll region an accessible focus target/label and verify Arrow/Page scrolling with an overflowing excerpt.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
Supplemental blocking finding from the systems/payload pass at head 36a1c549. This is additive to review 4990012073.
| }; | ||
| return [ | ||
| CONTEXT_PREFIX, | ||
| `${FRAME_PREFIX}${JSON.stringify(frame)}`, |
There was a problem hiding this comment.
P1 · Include serialized quotes in the ACP frame budget
This adds every complete excerpt to the outgoing JSON-RPC message without any length cap or byte accounting. The existing safety guard counts only image base64 (promptAttachmentBytes) and permits up to 12 MiB specifically to leave headroom beneath tungstenite's 16 MiB frame limit. A quote can consume that headroom—or exceed the limit by itself—after both send and steer have passed the image-only guard. acp.ts then appends this block and the images to the same client.prompt / extMethod payload.
User effect: a sufficiently large quote, or a moderate quote combined with attachments near the allowance, can overflow the transport frame and drop the shared ACP WebSocket/every open chat despite the safety gate.
Budget the serialized full ACP request (prompt, quote block, assistant content, images, metadata/envelope, and safety headroom) before local commit/dispatch in both send and steer. Test quote-only overflow and a near-budget image plus quote; assert rejection before directAcp.prompt / steerSession and before a local user message is committed.
2 participants
Overview
Category: new feature
User impact: Users can select visible text from a user or agent message and attach it as structured context to their next message.
Long responses are difficult to discuss precisely. Referring to “the third bullet” is ambiguous, while copying text into the composer loses the distinction between the user’s words and the passage they are discussing.
This PR adds a Quote in message action. A quote remains separate from typed text as a removable composer chip, appears as a quote card on the sent user message, and is restored with that user turn during replay.
Architecture
The implementation is rendered excerpt first:
The complete excerpt always travels with the next user turn as user-provided context, never as a system-level or assistant instruction. Collision-safe framing prevents selected text from escaping its quote boundary.
Replay follows the existing skill-chip pattern: the quote context is stored as an assistant-visible block in the same user turn, recognized by its authoritative message ID, hidden from transcript prose, and reconstructed as quote-card metadata.
Product behavior
What changed
Verification
pnpm checkpnpm typecheckManual test plan